feat: add CI verification targets and PR checks - #41
Merged
Conversation
- Rename .eslintrc.js to .eslintrc.cjs (ESM module compatibility) - Replace 17 removed formatting rules with @stylistic/eslint-plugin equivalents - Replace ban-types with no-restricted-types + no-unsafe-function-type + no-wrapper-object-types - Replace no-empty-interface with no-empty-object-type - Replace no-throw-literal with only-throw-error - Remove no-var-requires (covered by no-require-imports) - Remove prefer-ts-expect-error (covered by ban-ts-comment) - Replace no-loss-of-precision with base ESLint rule - Fix lint target to point at ui/ instead of nonexistent src/ - Point parserOptions.project at tsconfig.app.json
Add Makefile targets for deterministic local/CI verification: - make check: runs all checks in sequence (fail-fast) - Go: go-build, go-vet, go-test, go-lint, fmt-check, fmt - Bindings: bindings, bindings-check (detects stale wails bindings) - UI: ui-install, ui-build, ui-lint, ui-typecheck Add GitHub Actions PR workflow (.github/workflows/pr-checks.yml): - Runs Go checks, bindings freshness, and UI checks in parallel - Caches Go modules and pnpm store - Gates PR merges on all checks passing Add dist/.gitkeep so go:embed all:dist resolves without a frontend build, enabling Go checks to run independently.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReorganizes CI and linting, adds a PR GitHub Actions workflow, extends Makefile CI targets, updates ESLint configuration and package scripts, ensures plugin PID test directories exist, tweaks runtime relationship model fields, and adjusts .gitignore to keep Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant GH as GitHub Actions
participant Runner as Runner (ubuntu-latest)
participant Make as Makefile
participant Go as Go toolchain
participant Wails as Wails tooling
participant Node as Node/PNPM
PR->>GH: push PR triggers "PR Checks"
GH->>Runner: start job (go-checks)
Runner->>Make: run make go-build / go-vet / go-test / fmt-check
Make->>Go: invoke go build/vet/test/format
GH->>Runner: start job (bindings-check)
Runner->>Make: run make bindings-check
Make->>Wails: run bindings verification
GH->>Runner: start job (ui-checks)
Runner->>Make: run make ui-install / ui-build / ui-lint / ui-typecheck
Make->>Node: install/build/lint/typecheck via PNPM/Node
Runner-->>GH: report job statuses
GH-->>PR: post checks results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Fix pids_test.go: ensure ~/.omniview/ directory exists before tests that write to the PID file (fails on CI runners without home setup) - Regenerate stale wails bindings (models.ts was out of sync with Go structs: added direction, targetNamespaced fields; removed ownerRefKind) - Remove --max-warnings 0 from lint script (1523 pre-existing errors) - Mark fmt-check, go-lint, ui-lint, ui-typecheck as continue-on-error in CI workflow (pre-existing issues on main, will be fixed separately)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-checks.yml:
- Around line 7-9: The concurrency group key "pr-checks-${{ github.head_ref }}"
is not PR-unique; update the key to include a PR-unique identifier (for example
use github.event.pull_request.number or include the head repository) so runs
from different forks/PRs with the same branch name do not collide; locate the
concurrency block (the "concurrency" group setting) and change the group string
to include either "${{ github.event.pull_request.number }}" or "${{
github.head_repo }}" (or both) to make the key unique per PR.
- Around line 41-45: Replace the non-deterministic version pin in the GitHub
Actions "Lint" step that uses golangci/golangci-lint-action (currently set as
version: latest) with a specific release tag (for example v2.10 or another
tested release) to ensure reproducible CI; update the version field in the Lint
step of the workflow so the action reference remains the same
(golangci/golangci-lint-action@v6) but the with.version value is a fixed tag
instead of "latest".
In `@backend/pkg/plugin/pids_test.go`:
- Around line 47-49: Extract the repeated PID-directory setup into a test
helper: create a new function ensurePluginPIDDir(t *testing.T) that calls
require.NoError(t, os.MkdirAll(filepath.Dir(pluginPIDFilePath()), 0755)) and use
it in tests that currently duplicate that line (replace occurrences around
pluginPIDFilePath in pids_test.go, e.g., the blocks at lines referenced in the
review). Update all tests to call ensurePluginPIDDir(t) before operating on PID
files so setup is consistent and reduces duplication.
In `@Makefile`:
- Around line 32-42: The fmt-check target is inconsistent with fmt: fmt runs
both goimports and gofmt but fmt-check only checks gofmt; update fmt-check to
mirror fmt by verifying both goimports and gofmt (or remove goimports from fmt).
Specifically, modify the Makefile's fmt-check target (referencing targets "fmt"
and "fmt-check" and the tools "goimports" and "gofmt") so it detects files that
would be modified by goimports as well as gofmt, and fail when either tool would
change files.
- Around line 48-57: The bindings-check recipe can mask failures and leave the
repo dirty; modify the bindings-check target so it starts with "set -e" to exit
on first error, create a trap handler that always removes TMPDIR and, on
failure, restores the backup copy at "$$TMPDIR/wailsjs-before" back to
packages/omniviewdev-runtime/src/wailsjs (or otherwise revert changes) before
exiting; ensure the trap runs for both success and error paths and that TMPDIR
is cleaned up so a failing "cp" or "wails generate module" cannot be hidden nor
leave modified bindings in place.
In `@package.json`:
- Line 11: The npm "lint" script currently ("lint": "eslint ui --ext ts,tsx
--report-unused-disable-directives") is missing the environment variable used by
the Makefile target; update the package.json "lint" script to set
ESLINT_USE_FLAT_CONFIG=false (or use a cross-platform wrapper) so pnpm lint
matches make ui-lint, or alternatively complete the ESLint flat-config migration
(remove .eslintrc.cjs and add eslint.config.*) so the env var is no longer
required; target the "lint" script entry in package.json and the presence of
.eslintrc.cjs when implementing the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c1b34d5f-3e4c-4498-9e36-3f64d8ec74f7
⛔ Files ignored due to path filters (2)
dist/.gitkeepis excluded by!**/dist/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.eslintrc.cjs.github/workflows/pr-checks.yml.gitignoreMakefilebackend/pkg/plugin/pids_test.gopackage.jsonpackages/omniviewdev-runtime/src/wailsjs/go/models.tspackages/omniviewdev-runtime/src/wailsjs/runtime/package.jsonpackages/omniviewdev-runtime/src/wailsjs/runtime/runtime.d.tspackages/omniviewdev-runtime/src/wailsjs/runtime/runtime.js
Comment on lines
+48
to
+57
| bindings-check: | ||
| @TMPDIR=$$(mktemp -d); \ | ||
| cp -R packages/omniviewdev-runtime/src/wailsjs "$$TMPDIR/wailsjs-before"; \ | ||
| GOWORK=off wails generate module; \ | ||
| if ! diff -r packages/omniviewdev-runtime/src/wailsjs "$$TMPDIR/wailsjs-before" >/dev/null 2>&1; then \ | ||
| echo "Wails bindings are stale. Run 'make bindings' and commit the result."; \ | ||
| rm -rf "$$TMPDIR"; \ | ||
| exit 1; \ | ||
| fi; \ | ||
| rm -rf "$$TMPDIR" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf 'bindings-check recipe:\n'
sed -n '48,57p' Makefile
printf '\nShell exit-code demo:\n'
code=$(sh -c 'false; true'; printf '%s' "$?")
printf 'exit=%s\n' "$code"Repository: omniviewdev/omniview
Length of output: 519
🏁 Script executed:
# Check if there are other make recipes that might show similar patterns
grep -n "set -e\|&&\|trap" Makefile | head -20Repository: omniviewdev/omniview
Length of output: 117
🏁 Script executed:
# Check if wails generate is used elsewhere
rg "wails generate" -nRepository: omniviewdev/omniview
Length of output: 665
🏁 Script executed:
# Look at the entire Makefile context around the recipe
cat -n Makefile | sed -n '40,65p'Repository: omniviewdev/omniview
Length of output: 857
🏁 Script executed:
# Check if there's a bindings make recipe (likely related to this check)
grep -n "^bindings:" MakefileRepository: omniviewdev/omniview
Length of output: 76
🏁 Script executed:
bindings-check:
`@set` -e; \
TMPDIR=$$(mktemp -d); \
trap "rm -rf \"$$TMPDIR\"" EXIT; \
cp -R packages/omniviewdev-runtime/src/wailsjs "$$TMPDIR/wailsjs-before"; \
GOWORK=off wails generate module; \
if ! diff -r packages/omniviewdev-runtime/src/wailsjs "$$TMPDIR/wailsjs-before" >/dev/null 2>&1; then \
echo "Wails bindings are stale. Run 'make bindings' and commit the result."; \
exit 1; \
fiRepository: omniviewdev/omniview
Length of output: 229
🏁 Script executed:
trap "cp -R \"$$TMPDIR/wailsjs-before\" packages/omniviewdev-runtime/src/wailsjs; rm -rf \"$$TMPDIR\"" EXITRepository: omniviewdev/omniview
Length of output: 131
Use set -e and trap to prevent error masking and restore state on failure.
This recipe chains commands with semicolons without set -e, which masks failures: if cp or wails generate module fails, the error is hidden by subsequent commands returning 0. Additionally, the recipe modifies the working tree by running wails generate module in place (line 51), leaving bindings modified even when the check fails.
To fix this:
- Add
set -eto fail immediately on any error - Use a
trapcleanup handler to ensurerm -rf "$$TMPDIR"and optionally restore the backup on error - Consider whether bindings should be regenerated in place during a check, or whether the backup should be restored before exiting on failure
This prevents silent failures and ensures the working tree is not dirtied when bindings are stale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 48 - 57, The bindings-check recipe can mask failures
and leave the repo dirty; modify the bindings-check target so it starts with
"set -e" to exit on first error, create a trap handler that always removes
TMPDIR and, on failure, restores the backup copy at "$$TMPDIR/wailsjs-before"
back to packages/omniviewdev-runtime/src/wailsjs (or otherwise revert changes)
before exiting; ensure the trap runs for both success and error paths and that
TMPDIR is cleaned up so a failing "cp" or "wails generate module" cannot be
hidden nor leave modified bindings in place.
- Use PR number in concurrency group key to avoid cross-fork collisions - Pin golangci-lint version to v2.1 instead of latest - Extract ensurePluginPIDDir helper to deduplicate test setup - Make fmt-check verify both goimports and gofmt (matching fmt target) - Add set -e and trap cleanup to bindings-check so failures restore the original bindings and always clean up the temp directory - Move ESLINT_USE_FLAT_CONFIG=false into package.json lint script so pnpm lint and make ui-lint behave identically
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-checks.yml:
- Line 14: Update the workflow environment version pins to exactly match
.tool-versions: change PNPM_VERSION to 9.15.0, NODE_VERSION to 20.18.1, and
GO_VERSION to 1.23.8 (update the PNPM_VERSION, NODE_VERSION and GO_VERSION
variables used in the workflow file so CI and local dev use the same tool
versions).
In `@Makefile`:
- Around line 36-45: The fmt-check target currently skips import validation when
goimports is missing; update the fmt-check rule (and align with the existing fmt
target behavior) to require goimports by failing loudly if command -v goimports
returns non-zero—i.e., detect absence of goimports and echo an error + exit
non-zero (or invoke go install golang.org/x/tools/cmd/goimports) before running
goimports -l ., so CI cannot silently pass when goimports is not installed;
refer to the fmt-check target and the fmt target when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2ad855b9-e965-41bc-a55b-3409740ac1ce
📒 Files selected for processing (4)
.github/workflows/pr-checks.ymlMakefilebackend/pkg/plugin/pids_test.gopackage.json
| env: | ||
| GO_VERSION: "1.26" | ||
| NODE_VERSION: "20" | ||
| PNPM_VERSION: "10" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo ".tool-versions:"
sed -n '1,5p' .tool-versions
echo
echo "Workflow env versions:"
sed -n '11,15p' .github/workflows/pr-checks.ymlRepository: omniviewdev/omniview
Length of output: 213
Align all environment versions with .tool-versions.
Version mismatches between workflow and .tool-versions can cause local/CI drift:
- PNPM: workflow pins
10,.tool-versionshas9.15.0 - NODE: workflow pins
20,.tool-versionshas20.18.1 - GO: workflow pins
1.26,.tool-versionshas1.23.8(most significant drift)
Update all three to match .tool-versions for consistency.
💡 Example fix
env:
- GO_VERSION: "1.26"
- NODE_VERSION: "20"
- PNPM_VERSION: "10"
+ GO_VERSION: "1.23.8"
+ NODE_VERSION: "20.18.1"
+ PNPM_VERSION: "9.15.0"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-checks.yml at line 14, Update the workflow environment
version pins to exactly match .tool-versions: change PNPM_VERSION to 9.15.0,
NODE_VERSION to 20.18.1, and GO_VERSION to 1.23.8 (update the PNPM_VERSION,
NODE_VERSION and GO_VERSION variables used in the workflow file so CI and local
dev use the same tool versions).
- Pin NODE_VERSION to 20.18.1 and PNPM_VERSION to 9.15.0 to match .tool-versions (GO_VERSION stays at 1.26 per go.mod requirement) - Make fmt-check fail loudly when goimports is not installed instead of silently skipping the import check
This was referenced Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
make checkruns all: go-build, go-vet, go-test, go-lint, fmt-check, bindings-check, ui-install, ui-build, ui-lint, ui-typecheck).github/workflows/pr-checks.yml) that runs Go, bindings, and UI checks in parallel with cachingdist/.gitkeepsogo:embed all:distresolves without a frontend buildui/instead of nonexistentsrc/make checkas standard verificationTicket
Feature: Add CI-style verification targets and PR checks for omniview
Verification
All individual Makefile targets were tested locally:
make go-build— passes (with GOWORK=off + dist/.gitkeep)make go-vet— passesmake go-test— all tests passmake go-lint— runs correctly (pre-existing lint issues in codebase)make fmt-check— correctly detects unformatted Go filesmake bindings-check— correctly detects stale wails bindingsmake ui-install— passes with --frozen-lockfilemake ui-build— passes (packages + frontend)make ui-lint— runs correctly after eslint config migration (pre-existing lint issues)make ui-typecheck— runs correctly (pre-existing type errors)Notes
bindings-checktarget detected that the committed wails bindings on main are stale (missingdirectionandtargetNamespacedfields in RelationshipDescriptor)Summary by CodeRabbit
Chores
Tests
Chores